tileset tab in examples #46
Conversation
|
@juandjara is this ready for review? I see you've assigned us as reviewers, but it's still marked as a draft and CI is not yet passing, so I'm removing this from my review list for now - please feel free to update the PR and re-assign me whenever you're ready! |
| onViewportLoad(tiles) { | ||
| data?.then((res) => { | ||
| setTilesLoaded(true) | ||
| res.widgetSource.loadTiles(tiles) | ||
| const bbox = new WebMercatorViewport(viewState).getBounds() | ||
| const spatialFilter = createViewportSpatialFilter(bbox) | ||
| if (spatialFilter) { | ||
| res.widgetSource.extractTileFeatures({ spatialFilter }) | ||
| } | ||
| }) | ||
| }, | ||
| }), | ||
| ]; | ||
| }, [data, layerVisibility]); |
There was a problem hiding this comment.
Missing a useMemo dependency for viewState here – the tiles may be filtered by an old viewport, returning incorrect or missing data.
The other thing to consider is that the 'onViewportLoad' event could happen either before or after the (debounced) 'onViewStateChange' event, when the user moves the map. If 'onViewportLoad' is second, then there is nothing that cues the widgets to recalculate, and they'll keep showing old results.
We could fix that by adding an imperative .refresh() method to the widgets, but that feels like "bad style" in React... maybe the simplest thing is just to force-update the viewState once more, after tiles load? Then we can remove the extractTileFeatures call here, and most of the time avoid filtering twice:
onViewportLoad(tiles) {
data?.then((res) => {
setTilesLoaded(true);
res.widgetSource.loadTiles(tiles);
setViewState({ ...viewState }); // force refresh
});
}There was a problem hiding this comment.
Great! This fixes it!
I think the initial using of data.then everywhere is what threw me off when reading the async flows as I just copied that from other examples. But this seems to work so great
No description provided.